Skip to content

[CS2113-T15-1] CLIAlgo#33

Open
ong-ck wants to merge 772 commits into
nus-cs2113-AY2223S2:masterfrom
AY2223S2-CS2113-T15-1:master
Open

[CS2113-T15-1] CLIAlgo#33
ong-ck wants to merge 772 commits into
nus-cs2113-AY2223S2:masterfrom
AY2223S2-CS2113-T15-1:master

Conversation

@ong-ck

@ong-ck ong-ck commented Mar 2, 2023

Copy link
Copy Markdown

CLIAlgo is a desktop application for managing your CS2040C notes. It is optimized to be used via a Command Line Interface (CLI). If you can type fast, you can access and sort your notes faster than ever before.

@chao2048 chao2048 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DG is pretty nice and clear overall!

Comment thread docs/DeveloperGuide.md Outdated
### Initializing previous saved data feature
#### Current implementation

![](.\\sequence\\diagrams\\InitializationFileManager.png "FileManager Initialization Sequence Diagram")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the activation bar for the FileDecoder a bit too long?

Comment thread docs/DeveloperGuide.md Outdated
### Export feature
#### Current implementation

![](.\\sequence\\diagrams\\Export.png "Export Sequence Diagram")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can explain more about the part of Parser to FilterByTopicCommand to TopicManager, since they are shown on the diagram. Or perhaps it is not that important and can be ignored in the sequence diagram.

Comment thread docs/DeveloperGuide.md Outdated
Here is a class diagram of the `TopoCommand` which facilitates the storage
function of the application.

![](.\\uml\\diagrams\\TopoCommandClass.png "TopoCommand Class Diagram")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FileManager doesn't seem to be very relevant to the class diagram, and it makes the lines crossing each other. Perhaps it doesn't have to be in this diagram.

Comment thread docs/DeveloperGuide.md
#### Current implementation

![](.\\sequence\\diagrams\\InitializationFileManager.png "FileManager Initialization Sequence Diagram")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might need an arrow back from SingleFile back to FileManager after the readFile().

@GraceZhuXY GraceZhuXY left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DG is detailed and easy to read! Great job 👍

Comment thread docs/DeveloperGuide.md Outdated

### Ui
**API** : Ui.java
Here is a class diagram of the `Ui` component which is responsible for handling all interactin with the User.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be a small typo here ("interactin")

Comment thread docs/DeveloperGuide.md Outdated

The following sequence diagram shows how the Parser work.

![](.\\sequence\\diagrams\\Parser.png "Parser Sequence Diagram")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The step-by-step description of the Parser is easy to understand! Perhaps the sequence diagram can also be simplified such that it is easier to comprehend together with the description (e.g. by removing return arrows, which is easy to infer)? This could also be applied to the sequence diagrams for filtering by topic operation.

Comment thread docs/DeveloperGuide.md Outdated
> **Step 2**: The `parse()` method extracts out the command keyword provided by the user. It then calls the
> `prepareCommand()` method.

> **Step 3**: The `parepareCommand()` identifies the correct `Command` object to prepare based on the command keyword.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be a small typo here ("parepareCommand()")

Comment thread docs/DeveloperGuide.md
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the
original source as well}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be great if there was a link to your UG/a brief overview of what this app is programmed to do! It would make it easier see all the following technical details in the bigger picture.

Comment thread docs/DeveloperGuide.md Outdated
Here is a class diagram of the `FileManager` which facilitates the storage
function of the application.

![](.\\uml\\diagrams\\FileManagerClass.png "FileManager Class Diagram")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the diagram's focus would be clearer without the Scanner and FileWriter classes, as their functions can be described using labels.

Comment thread docs/DeveloperGuide.md
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the
original source as well}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to include a table of contents and links to the headers for easier navigation

Comment thread docs/DeveloperGuide.md Outdated

The following sequence diagram shows how the Parser work.

![](.\\sequence\\diagrams\\Parser.png "Parser Sequence Diagram")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Good job on the comprehensiveness of the diagram but the picture is unclear. maybe you can try dissecting this part into smaller sections

Comment thread docs/DeveloperGuide.md Outdated

The following sequence diagram shows how previously saved files are loaded into `CLIAlgo`.

![](.\\sequence\\diagrams\\InitializationFileManager.png "FileManager Initialization Sequence Diagram")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

i think the activation bar inside the alt block should not be broken into two parts

Comment thread docs/uml/diagrams/TopoCommandClass.png Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be try to modify the diagram to ensure the lines are not crossing each other

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The activation bar of [isCorrupted] should not be disconnected

Comment thread docs/DeveloperGuide.md Outdated
## Acknowledgements

{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this section have links to your references?

Comment thread docs/sequence/diagrams/AddFeature.png Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the AddCommand and InvalidComment be shown at the end of the diagram when a red cross deletes the instance? This applied similarly to your other sequence diagrams.

Comment thread docs/UML/diagrams/ParserClass.png Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the command class also have two blank sections for attribute and method like other classes?

Comment thread docs/UserGuide.md

1. Ensure that you have Java 11 or above installed.
1. Down the latest version of `Duke` from [here](http://link.to/duke).
## Features

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the features section be placed in the user guide instead?

nicholas132000 and others added 24 commits April 3, 2023 11:22
…into branch-v2.0b-DG

# Conflicts:
#	docs/sequence-diagrams/diagrams/AddFeature.png
#	docs/sequence-diagrams/source-files/AddFeature.puml
Add design and implementation for remove and update add sequence diagram
heejet and others added 30 commits April 9, 2023 15:29
Final Refactor of UG, DG and PPP v2.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants